Split out commands so we can just import host commands from realm defined commands by tintinthong · Pull Request #4432 · cardstack/boxel

tintinthong · 2026-04-17T01:46:50Z

No description provided.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7ebe1ca5f0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-17T01:51:38Z

+    const { results: atomicResults } = await new ExecuteAtomicOperationsCommand(
+      this.commandContext,
+    ).execute({ realmUrl, operations });


Preserve stale-spec guidance on atomic install failures

When execute-atomic-operations fails during listing install, the error is now propagated verbatim. This drops the listing-specific handling for the known stale-spec case (filter refers to a nonexistent type), so users get a low-level backend error instead of the actionable "Update Specs" instruction and can’t recover from install failures without manual debugging.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-17T01:51:38Z

+        if (!doc || !('data' in doc)) {
          throw new Error('We are only expecting single documents returned');
        }
-        delete doc.data.id;
-        delete doc.included;
-        let cardResource: LooseCardResource = doc?.data;
+        delete (doc as any).data.id;


Validate fetched card JSON as single-card document

This check only verifies that a data key exists, then treats doc.data as a single card resource. If FetchCardJsonCommand returns a collection-style JSONAPI payload (data array) or another non-single shape, we will construct invalid atomic add operations instead of failing early, which makes installs fail later with harder-to-diagnose errors.

Useful? React with 👍 / 👎.

github-actions · 2026-04-17T03:00:26Z

Preview deployments

github-actions · 2026-04-17T03:14:23Z

Realm Server Test Results

1 files ±0 1 suites ±0 14m 33s ⏱️ -15s
894 tests ±0 894 ✅ ±0 0 💤 ±0 0 ❌ ±0
966 runs ±0 966 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit f39aebd. ± Comparison against base commit 9b7a4b5.

♻️ This comment has been updated with latest results.

github-actions · 2026-04-17T03:20:27Z

Host Test Results

2 277 tests +19 2 262 ✅ +19 2h 32m 31s ⏱️ + 1m 34s
1 suites ± 0 15 💤 ± 0
1 files ± 0 0 ❌ ± 0

Results for commit f39aebd. ± Comparison against base commit 9b7a4b5.

This pull request removes 1 and adds 20 tests. Note that renamed tests count towards both.

Chrome ‑ Integration | commands | search-and-choose: uses sourceContextCodeRef source instead of the searched type source

Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for AuthedFetchCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for CanReadRealmCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for ExecuteAtomicOperationsCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for FetchCardJsonCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for GetAvailableRealmUrlsCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for GetCatalogRealmUrlsCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for GetDefaultWritableRealmCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for GetRealmOfUrlCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for PersistModuleInspectorViewCommand
Chrome ‑ Integration | Command | host command schema generation test > command schema generation: getInputJsonSchema for SanitizeModuleListCommand
…

♻️ This comment has been updated with latest results.

Copilot

Pull request overview

This PR splits several host-side behaviors into dedicated HostBaseCommand implementations so that realm-defined commands can import and invoke host commands (via the host command shims) instead of reaching into host services directly.

Changes:

Introduces new host commands for shared operations (realm validation, authed fetch, module-list sanitization, store add, atomic ops execution, etc.) and registers them in the host command shim/index.
Refactors multiple listing-related commands to call these new commands rather than injecting host services.
Extends packages/base/command.gts with new input/result card definitions for the added commands.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
packages/host/app/commands/validate-realm.ts	New command to normalize and validate realm URLs against available realms.
packages/host/app/commands/store-add.ts	New command wrapper around store add behavior.
packages/host/app/commands/sanitize-module-list.ts	New shared dependency/module URL filtering + dedupe command.
packages/host/app/commands/persist-module-inspector-view.ts	New command wrapper to persist module inspector view selection.
packages/host/app/commands/listing-use.ts	Refactors realm validation to use `ValidateRealmCommand`.
packages/host/app/commands/listing-update-specs.ts	Refactors dependency fetching and sanitization to use `AuthedFetchCommand` + `SanitizeModuleListCommand`.
packages/host/app/commands/listing-remix.ts	Refactors realm validation + module inspector view persistence into commands.
packages/host/app/commands/listing-install.ts	Refactors realm validation, source/card fetch, atomic ops execution, and serialization to command wrappers.
packages/host/app/commands/listing-generate-example.ts	Uses `GetDefaultWritableRealmCommand` instead of direct RealmService access.
packages/host/app/commands/listing-create.ts	Refactors catalog realm discovery, dependency fetch/sanitize, store add, and card loading into commands.
packages/host/app/commands/listing-action-init.ts	Removes direct host service injections (part of command-splitting refactor).
packages/host/app/commands/listing-action-build.ts	Removes direct host service injections (part of command-splitting refactor).
packages/host/app/commands/index.ts	Registers/shims newly introduced host commands and adds them to `HostCommandClasses`.
packages/host/app/commands/get-realm-of-url.ts	New command to compute containing realm URL for an absolute URL.
packages/host/app/commands/get-default-writable-realm.ts	New command to expose default writable realm path.
packages/host/app/commands/get-catalog-realm-urls.ts	New command to expose catalog realm URL list.
packages/host/app/commands/get-available-realm-urls.ts	New command to expose available realm URL list.
packages/host/app/commands/fetch-card-json.ts	New command to fetch a card JSON document by URL.
packages/host/app/commands/execute-atomic-operations.ts	New command to execute atomic operations and return atomic results.
packages/host/app/commands/can-read-realm.ts	New command to check read permissions for a realm.
packages/host/app/commands/authed-fetch.ts	New command wrapper around authenticated fetch returning `{ok,status,body}`.
packages/base/command.gts	Adds new input/result card types needed by the new host commands.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-17T13:28:25Z

+    let writtenFiles = (atomicResults as Array<Record<string, any>>).map(
+      (r) => r.data?.id,
+    );


writtenFiles is derived via r.data?.id, which can yield undefined entries when the atomic-op result shape differs (or data is missing). Consider filtering out falsy ids (or validating the result shape) before logging so the debug output accurately reflects what was written.

Suggested change

let writtenFiles = (atomicResults as Array<Record<string, any>>).map(

(r) => r.data?.id,

);

let writtenFiles = (atomicResults as Array<Record<string, any>>)

.map((r) => r.data?.id)

.filter((id): id is string => typeof id === 'string' && id.length > 0);

Copilot · 2026-04-17T13:28:26Z

+    this.operatorModeStateService.persistModuleInspectorView(
+      input.codePath,
+      input.moduleInspectorView as 'schema' | 'spec' | 'preview',
+    );


moduleInspectorView is cast to 'schema' | 'spec' | 'preview' without runtime validation. Since the input card field is a plain StringField, an unexpected value can still be passed through and persisted to localStorage, potentially breaking later reads of the selection. Consider validating the input against the allowed set and throwing a clear error for invalid values instead of using a type cast.

Copilot · 2026-04-17T13:28:26Z

+      const normalized = trimExecutableExtension(new URL(m)).href;
+      if (!seen.has(normalized)) {
+        seen.set(normalized, m);


SanitizeModuleListCommand currently assumes every entry in input.moduleUrls is a valid absolute URL. new URL(m) will throw on any invalid/relative value, which will abort the entire command rather than filtering the bad entry (the previous inline sanitizers used try/catch per dep). Consider wrapping URL parsing/normalization in a per-item try/catch and skipping invalid module URLs instead of failing the whole command.

Suggested change

const normalized = trimExecutableExtension(new URL(m)).href;

if (!seen.has(normalized)) {

seen.set(normalized, m);

try {

const normalized = trimExecutableExtension(new URL(m)).href;

if (!seen.has(normalized)) {

seen.set(normalized, m);

}

} catch {

// Skip invalid or relative module URLs during sanitization

Copilot · 2026-04-17T13:28:26Z

+    let body: Record<string, any> = {};
+    if (response.ok) {
+      try {
+        body = await response.json();
+      } catch {
+        // non-JSON response
+      }
+    }


AuthedFetchCommand swallows JSON parse errors and still returns { ok: true, body: {} } when response.ok but the body isn't valid JSON. This can mask server issues and lead callers to proceed with an empty object. Consider either (a) returning the raw text when JSON parsing fails, (b) exposing a parseError/isJson flag, or (c) treating "ok" as false (or throwing) when an acceptHeader indicates the caller expects JSON but parsing fails.

Copilot · 2026-04-17T13:28:27Z

+  ): Promise<BaseCommandModule.GetRealmOfUrlResult> {
+    let commandModule = await this.loadCommandModule();
+    const { GetRealmOfUrlResult } = commandModule;
+    const realmUrl = this.realm.realmOfURL(new URL(input.url));


GetRealmOfUrlCommand calls new URL(input.url) without handling parse failures. Since this command is used as part of module/dependency sanitization, a single invalid/relative URL string would throw and interrupt the whole flow. Consider catching URL construction errors and returning an empty realmUrl (or a non-ok result) instead of throwing.

Suggested change

const realmUrl = this.realm.realmOfURL(new URL(input.url));

let parsedUrl: URL;

try {

parsedUrl = new URL(input.url);

} catch (_error) {

return new GetRealmOfUrlResult({ realmUrl: '' });

}

const realmUrl = this.realm.realmOfURL(parsedUrl);

Copilot · 2026-04-17T13:28:27Z

@@ -141,13 +107,16 @@ export default class ListingCreateCommand extends HostBaseCommand<
        relationships,
        meta: {
          adoptsFrom: {
-            module: `${this.catalogRealm}catalog-app/listing/listing`,
+            module: `${catalogRealm}catalog-app/listing/listing`,
            name: listingSubClass[listingType],
          },
        },


getCatalogRealm() can return undefined, but the result is interpolated into the adoptsFrom module URL. If no catalog realm is found, this will generate a malformed URL like undefinedcatalog-app/... and lead to hard-to-debug downstream errors. Consider validating catalogRealm and throwing a clear error when it is missing (or falling back to a known default).

Copilot · 2026-04-17T13:28:27Z

+        let { document: doc } = await new FetchCardJsonCommand(
+          this.commandContext,
+        ).execute({ url: sourceCard.id });
+        if (!doc || !('data' in doc)) {
          throw new Error('We are only expecting single documents returned');
        }
-        delete doc.data.id;
-        delete doc.included;
-        let cardResource: LooseCardResource = doc?.data;
+        delete (doc as any).data.id;
+        delete (doc as any).included;
+        let cardResource: LooseCardResource = (doc as any)
+          .data as LooseCardResource;


The new document-shape check (!doc || !('data' in doc)) is much weaker than the previous isSingleCardDocument validation. This can allow non–single-card documents (or unexpected shapes) through and then build atomic operations with invalid data, causing server-side failures or corrupted writes. Consider restoring isSingleCardDocument (or an equivalent structural check) before mutating/using doc.data.

Copilot · 2026-04-17T13:28:27Z

+    const { results: atomicResults } = await new ExecuteAtomicOperationsCommand(
+      this.commandContext,
+    ).execute({ realmUrl, operations });



ListingInstallCommand previously translated a known atomic-ops failure (filter refers to a nonexistent type) into a more actionable, user-facing error. With the refactor to ExecuteAtomicOperationsCommand, that special-case is lost and users may now see a low-level server detail. Consider reintroducing that mapping either here (around the execute call) or inside ExecuteAtomicOperationsCommand so the UX doesn't regress.

habdelra

probably we should have dedicated tests for the new lower level commands

split out listing commands

7ebe1ca

chatgpt-codex-connector bot reviewed Apr 17, 2026

View reviewed changes

tintinthong added 2 commits April 17, 2026 10:51

add commands

cb912af

Merge branch 'main' into split-out-commands

0a561ea

tintinthong added 5 commits April 17, 2026 11:31

remove catalog test

eade780

normalize fetches to account for @cardstack/catalog

9ad716e

fix lint

a64e234

can we possibly remove the boxel-icons urls tht was added pls

1fb5e5b

remove all boxel-icons

f1323e4

tintinthong requested review from a team and richardhjtan April 17, 2026 07:29

fix lint

f39aebd

habdelra requested a review from Copilot April 17, 2026 13:21

Copilot started reviewing on behalf of habdelra April 17, 2026 13:21 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

habdelra reviewed Apr 17, 2026

View reviewed changes

-    const realmUrl = this.realm.realmOfURL(new URL(input.url));
+    let parsedUrl: URL;
+    try {
+      parsedUrl = new URL(input.url);
+    } catch (_error) {
+      return new GetRealmOfUrlResult({ realmUrl: '' });
+    }
+    const realmUrl = this.realm.realmOfURL(parsedUrl);

Conversation

tintinthong commented Apr 17, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 17, 2026

Preview deployments

Uh oh!

github-actions bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Realm Server Test Results

Uh oh!

github-actions bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Host Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

habdelra left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Apr 17, 2026 •

edited

Loading

github-actions bot commented Apr 17, 2026 •

edited

Loading